-
Notifications
You must be signed in to change notification settings - Fork 164
Search handler and client with HTTP POST-based request serialization. #8892
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Future<shelf.Response> _debugHandler(shelf.Request request) async { | ||
final info = await searchIndex.indexInfo(); | ||
return debugResponse(info.toJson()); | ||
} | ||
|
||
/// Handles /search requests. | ||
/// Handles GET /search requests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to contradict the code below that branches on the method....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated comment, it should handle both.
@@ -53,7 +53,6 @@ enum SearchOrder { | |||
/// WARNING: The value shouldn't be used anymore. | |||
/// | |||
/// TODO: remove in a future release. | |||
@Deprecated('Popularity is no longer used.') | |||
popularity, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Intentional, but more of a convenience: if we keep it, the generated code uses it and analyzer complains about using a deprecated field. I did not want to remove it yet, but maybe we should?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the field. I think we did have enough time since we have changed from popularity to downloads. Old bookmarks may break, but that is going to happen no matter when we remove it.
TextMatchExtent
topkg/_pub_shared
, and added the new data class there, as the build command's inclusion rules otherwise couldn't identify all the required types./search
, since we have so little change based on the HTTP methodwithRetryHttpClient
to reuse some of its logic